Repository navigation
feat(mount-provider): implement IPartialMountProvider - #2378
Conversation
|
@provokateurin I've addressed your review comments and updated the PR description with new testing instructions. Thanks! |
a1632ed to
ce45026
Compare
|
@provokateurin @salmart-dev I've adjusted the PR after your suggestions. Would appreciate another review when you have a chance. |
|
provokateurin
left a comment
There was a problem hiding this comment.
LGTM, just one small change needed.
Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
…single query Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
…s function Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
f82630b to
523499a
Compare
|
Let's wait to merge this PR, I want to make another round of testitng to check things are working as expected. |
|
I set up a globalscale env with 1 master + 2 slaves + lookup server to check how circles currently generates mountpoints. slave 1 instance - On
That generated the following entries on oc_circles_mount:
oc_circles_mountpoint:
As seen above, circles stores mountpoints relatively, without any parent-child relationship in the database. Given that, the |
|
I agree that this seems to not work then, which is quite confusing. Please check how the full mountpoint is assembled for the folders and replicate it such that the query will match. Given the difficulties, it would be great if you could create a test for this. |
You mean to check how the mountpoints are generated on the
cristian@tuxedo:~/Dev/haze$ haze haze-particular-trumpeter occ circles:remote
+------------------------------------------------+----------+---------+--------------------+--------------------+---------+
| Instance | Type | iface | UID | Authed | Aliases |
+------------------------------------------------+----------+---------+--------------------+--------------------+---------+
| outspoken-greenfinch.haze.192.168.0.4.sslip.io | External | frontal | a0c388d0b30be62ed0 | a0c388d0b30be62ed0 | [] |
+------------------------------------------------+----------+---------+--------------------+--------------------+---------+
cristian@tuxedo:~/Dev/haze$ haze haze-outspoken-greenfinch occ circles:remote
+------------------------------------------------+----------+---------+--------------------+--------------------+---------+
| Instance | Type | iface | UID | Authed | Aliases |
+------------------------------------------------+----------+---------+--------------------+--------------------+---------+
| particular-trumpeter.haze.192.168.0.4.sslip.io | External | frontal | 1aebf1ffb540f8084a | 1aebf1ffb540f8084a | [] |
+------------------------------------------------+----------+---------+--------------------+--------------------+---------+
Entries were generated on {
"reqId": "gw45DAMrZbsKbl76fcc0",
"level": 2,
"time": "2026-05-14T11:45:52+00:00",
"remoteAddr": "172.19.0.1",
"user": "admin_particular",
"app": "PHP",
"method": "GET",
"url": "/index.php/apps/files/",
"scriptName": "/index.php",
"message": "Undefined array key \"manager\" at /var/www/html/apps/files_sharing/lib/External/Storage.php#62",
"userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/148.0.0.0 Safari/537.36",
"version": "34.0.0.6",
"data": {
"app": "PHP"
}
}{
"reqId": "gw45DAMrZbsKbl76fcc0",
"level": 3,
"time": "2026-05-14T11:45:52+00:00",
"remoteAddr": "172.19.0.1",
"user": "admin_particular",
"app": "index",
"method": "GET",
"url": "/index.php/apps/files/",
"scriptName": "/index.php",
"message": "Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager in file '/var/www/html/apps/files_sharing/lib/External/Storage.php' line 62",
"userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/148.0.0.0 Safari/537.36",
"version": "34.0.0.6",
"exception": {
"Exception": "Exception",
"Message": "Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager in file '/var/www/html/apps/files_sharing/lib/External/Storage.php' line 62",
"Code": 0,
"Trace": [
{
"file": "/var/www/html/lib/private/AppFramework/App.php",
"line": 137,
"function": "dispatch",
"class": "OC\\AppFramework\\Http\\Dispatcher",
"type": "->",
"args": [
{
"__class__": "OCA\\Files\\Controller\\ViewController"
},
"index"
]
},
{
"file": "/var/www/html/lib/private/Route/Router.php",
"line": 324,
"function": "main",
"class": "OC\\AppFramework\\App",
"type": "::",
"args": [
"OCA\\Files\\Controller\\ViewController",
"index",
{
"__class__": "OC\\AppFramework\\DependencyInjection\\DIContainer"
},
{
"_route": "files.view.index"
}
]
},
{
"file": "/var/www/html/lib/base.php",
"line": 1159,
"function": "match",
"class": "OC\\Route\\Router",
"type": "->",
"args": [
"/apps/files/"
]
},
{
"file": "/var/www/html/index.php",
"line": 25,
"function": "handleRequest",
"class": "OC",
"type": "::",
"args": []
}
],
"File": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php",
"Line": 110,
"Previous": {
"Exception": "TypeError",
"Message": "Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager",
"Code": 0,
"Trace": [
{
"file": "/var/www/html/lib/private/Files/Mount/MountPoint.php",
"line": 129,
"function": "__construct",
"class": "OCA\\Files_Sharing\\External\\Storage",
"type": "->",
"args": [
{
"0": "And 2 more entries, set log level to debug to see all entries",
"owner": "admin_outspoken",
"remote": "https://outspoken-greenfinch.haze.192.168.0.4.sslip.io",
"token": "",
"password": "",
"mountpoint": "/admin_particular/files/folder_outspoken"
}
]
},
{
"file": "/var/www/html/lib/private/Files/Mount/MountPoint.php",
"line": 155,
"function": "createStorage",
"class": "OC\\Files\\Mount\\MountPoint",
"type": "->",
"args": [
"*** sensitive parameters replaced ***"
]
},
{
"file": "/var/www/html/lib/private/Files/Mount/MountPoint.php",
"line": 263,
"function": "getStorage",
"class": "OC\\Files\\Mount\\MountPoint",
"type": "->",
"args": []
},
{
"file": "/var/www/html/lib/private/Files/Config/UserMountCache.php",
"line": 72,
"function": "getStorageRootId",
"class": "OC\\Files\\Mount\\MountPoint",
"type": "->",
"args": []
},
{
"file": "/var/www/html/lib/private/Files/SetupManager.php",
"line": 267,
"function": "registerMounts",
"class": "OC\\Files\\Config\\UserMountCache",
"type": "->",
"args": [
"*** sensitive parameters replaced ***",
[
{
"__class__": "OCA\\Circles\\MountManager\\CircleMount"
}
],
{
"0": "OC\\Files\\Mount\\CacheMountProvider",
"1": "OCA\\Circles\\MountManager\\CircleMountProvider",
"3": "OCA\\Files_Sharing\\External\\MountProvider"
}
]
},
{
"file": "/var/www/html/lib/private/Files/SetupManager.php",
"line": 483,
"function": "updateNonAuthoritativeProviders",
"class": "OC\\Files\\SetupManager",
"type": "->",
"args": [
"*** sensitive parameters replaced ***"
]
},
{
"file": "/var/www/html/lib/private/Files/Mount/Manager.php",
"line": 80,
"function": "setupForPath",
"class": "OC\\Files\\SetupManager",
"type": "->",
"args": [
"/admin_particular/files"
]
},
{
"file": "/var/www/html/lib/private/Files/View.php",
"line": 1441,
"function": "find",
"class": "OC\\Files\\Mount\\Manager",
"type": "->",
"args": [
"/admin_particular/files"
]
},
{
"file": "/var/www/html/lib/private/Files/Filesystem.php",
"line": 649,
"function": "getFileInfo",
"class": "OC\\Files\\View",
"type": "->",
"args": [
"/admin_particular/files",
false
]
},
{
"file": "/var/www/html/apps/files/lib/Controller/ViewController.php",
"line": 79,
"function": "getFileInfo",
"class": "OC\\Files\\Filesystem",
"type": "::",
"args": [
"/",
false
]
},
{
"file": "/var/www/html/apps/files/lib/Controller/ViewController.php",
"line": 172,
"function": "getStorageInfo",
"class": "OCA\\Files\\Controller\\ViewController",
"type": "->",
"args": [
"/"
]
},
{
"file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php",
"line": 165,
"function": "index",
"class": "OCA\\Files\\Controller\\ViewController",
"type": "->",
"args": [
"",
"",
null
]
},
{
"file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php",
"line": 78,
"function": "executeController",
"class": "OC\\AppFramework\\Http\\Dispatcher",
"type": "->",
"args": [
{
"__class__": "OCA\\Files\\Controller\\ViewController"
},
"index"
]
},
{
"file": "/var/www/html/lib/private/AppFramework/App.php",
"line": 137,
"function": "dispatch",
"class": "OC\\AppFramework\\Http\\Dispatcher",
"type": "->",
"args": [
{
"__class__": "OCA\\Files\\Controller\\ViewController"
},
"index"
]
},
{
"file": "/var/www/html/lib/private/Route/Router.php",
"line": 324,
"function": "main",
"class": "OC\\AppFramework\\App",
"type": "::",
"args": [
"OCA\\Files\\Controller\\ViewController",
"index",
{
"__class__": "OC\\AppFramework\\DependencyInjection\\DIContainer"
},
{
"_route": "files.view.index"
}
]
},
{
"file": "/var/www/html/lib/base.php",
"line": 1159,
"function": "match",
"class": "OC\\Route\\Router",
"type": "->",
"args": [
"/apps/files/"
]
},
{
"file": "/var/www/html/index.php",
"line": 25,
"function": "handleRequest",
"class": "OC",
"type": "::",
"args": []
}
],
"File": "/var/www/html/apps/files_sharing/lib/External/Storage.php",
"Line": 62
},
"message": "Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager in file '/var/www/html/apps/files_sharing/lib/External/Storage.php' line 62",
"exception": "{\"class\":\"Exception\",\"message\":\"Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager in file '/var/www/html/apps/files_sharing/lib/External/Storage.php' line 62\",\"code\":0,\"file\":\"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php:110\",\"trace\":\"#0 /var/www/html/lib/private/AppFramework/App.php(137): OC\\AppFramework\\Http\\Dispatcher->dispatch(Object(OCA\\Files\\Controller\\ViewController), 'index')\\n#1 /var/www/html/lib/private/Route/Router.php(324): OC\\AppFramework\\App::main('OCA\\\\Files\\\\Contr...', 'index', Object(OC\\AppFramework\\DependencyInjection\\DIContainer), Array)\\n#2 /var/www/html/lib/base.php(1159): OC\\Route\\Router->match('/apps/files/')\\n#3 /var/www/html/index.php(25): OC::handleRequest()\\n#4 {main}\",\"previous\":{\"class\":\"TypeError\",\"message\":\"Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager\",\"code\":0,\"file\":\"/var/www/html/apps/files_sharing/lib/External/Storage.php:62\",\"trace\":\"#0 /var/www/html/lib/private/Files/Mount/MountPoint.php(129): OCA\\Files_Sharing\\External\\Storage->__construct(Array)\\n#1 /var/www/html/lib/private/Files/Mount/MountPoint.php(155): OC\\Files\\Mount\\MountPoint->createStorage()\\n#2 /var/www/html/lib/private/Files/Mount/MountPoint.php(263): OC\\Files\\Mount\\MountPoint->getStorage()\\n#3 /var/www/html/lib/private/Files/Config/UserMountCache.php(72): OC\\Files\\Mount\\MountPoint->getStorageRootId()\\n#4 /var/www/html/lib/private/Files/SetupManager.php(267): OC\\Files\\Config\\UserMountCache->registerMounts(Object(OC\\User\\User), Array, Array)\\n#5 /var/www/html/lib/private/Files/SetupManager.php(483): OC\\Files\\SetupManager->updateNonAuthoritativeProviders(Object(OC\\User\\User))\\n#6 /var/www/html/lib/private/Files/Mount/Manager.php(80): OC\\Files\\SetupManager->setupForPath('/admin_particul...')\\n#7 /var/www/html/lib/private/Files/View.php(1441): OC\\Files\\Mount\\Manager->find('/admin_particul...')\\n#8 /var/www/html/lib/private/Files/Filesystem.php(649): OC\\Files\\View->getFileInfo('/admin_particul...', false)\\n#9 /var/www/html/apps/files/lib/Controller/ViewController.php(79): OC\\Files\\Filesystem::getFileInfo('/', false)\\n#10 /var/www/html/apps/files/lib/Controller/ViewController.php(172): OCA\\Files\\Controller\\ViewController->getStorageInfo('/')\\n#11 /var/www/html/lib/private/AppFramework/Http/Dispatcher.php(165): OCA\\Files\\Controller\\ViewController->index('', '', NULL)\\n#12 /var/www/html/lib/private/AppFramework/Http/Dispatcher.php(78): OC\\AppFramework\\Http\\Dispatcher->executeController(Object(OCA\\Files\\Controller\\ViewController), 'index')\\n#13 /var/www/html/lib/private/AppFramework/App.php(137): OC\\AppFramework\\Http\\Dispatcher->dispatch(Object(OCA\\Files\\Controller\\ViewController), 'index')\\n#14 /var/www/html/lib/private/Route/Router.php(324): OC\\AppFramework\\App::main('OCA\\\\Files\\\\Contr...', 'index', Object(OC\\AppFramework\\DependencyInjection\\DIContainer), Array)\\n#15 /var/www/html/lib/base.php(1159): OC\\Route\\Router->match('/apps/files/')\\n#16 /var/www/html/index.php(25): OC::handleRequest()\\n#17 {main}\"}}",
"CustomMessage": "Cannot assign null to property OCA\\Files_Sharing\\External\\Storage::$manager of type OCA\\Files_Sharing\\External\\Manager in file '/var/www/html/apps/files_sharing/lib/External/Storage.php' line 62"
}
}Seems like the manager key is missing on Since I couldn't get So it seems to me that the expected functionality is not yet working and I'm not sure if we should move on with this implementation, or try to fix the situation descbribed above first? Maybe I'm also missing something on the way I setup the environment? |
|
After investigation (details on previous comments) and internal discussion, it was decided to merge this PR for now as it looks in good shape from what was seen during step debugging. Problems faced during population of |
Summary
Implemented
IPartialMountProviderinCircleMountProviderby addinggetMountsForPath()and addedlimitToMountpoint()toCoreQueryBuilderto support filtering.Testing
I couldn't fully test this because the code path isn't triggered without changes to
lib/AppInfo/Application::registerMountProvider()andlib/FederatedItems/Files/FileShare::manage(). That said, I was able to check the behavior using step debugging as follows:if (!$this->configService->isLocalInstance($event->getOrigin()))condition inlib/FederatedItems/Files/FileShare::manage()to allow mount entries to be created locallymock_team_1,mock_team_2andmock_team_3mock_folder_1,mock_folder_2andmock_folder_3and shared with their respective teams (mock_team_1,mock_team_2andmock_team_3)returninlib/AppInfo/Application::registerMountProvider()to force registration ofCircleMountProviderregardless of GlobalScale availabilitycircles/lib/MountManager/CircleMountProvider::getMountsForUser(), accessednextcloud.local/index.php/apps/dashboard/, triggering thegetMountsForUser()methodgetMountsForPath()directly:Results
forChildrenwas set to false. With multiple folders on my setup (mock_team_1,mock_team_2andmock_team_3), in the test above only the requested mounts that exist in the DB were returned for the given paths when callinggetForUser()insidegetMountsForPath(), confirming thatlimitToMountpoints()is filtering correctly at the database level.forChildrendid not work as expected when set to true. During testing, circles always stored mountpoints relatively. For example, sharing a folder created insidemock_foldergenerated acircles_mountentry with mountpoint/mock_subfolderinstead of/mock_folder/mock_subfolder. As a result, theLIKE '/mock_folder/%'filter used whenforChildrenis true will not match these entries.generateCircleMount()threw an exception in both the originalgetMountsForUser()and on new methodgetMountsForPath(). I believe this happened because the mount entries were artificially created by bypassing the remote instance check inFileShare::manage(), resulting in incomplete data compared to what would be present in a real GlobalScale environment.Checklist
3. to review, feature component)stable32)AI (if applicable)